Skip to content

Conversation

@oyama
Copy link

@oyama oyama commented Apr 15, 2025

This PR fixes a boundary check error in tdbstore. When kv_set is called with data exactly equal to the available storage capacity, it erroneously returns MBED_ERROR_INVALID_DATA_DETECTED. The issue was traced to an off-by-one error in the boundary checking logic. The fix adjusts the condition so that data matching the exact storage boundary is now accepted.

The code to reproduce the problem is as follows:

#include "mbed.h"
#include "KVStore.h"
#include "kvstore_global_api.h"

int main()
{
    printf("kv_set boundary test\r\n");
    ThisThread::sleep_for(1000);

    // format kv store
    int res = kv_reset("/kv/");
    if (res != MBED_SUCCESS) {
        printf("kv_reset fail=%d\r\n", res);
        return 1;
    }

    // Write one area to near full
    char key[] = "/kv/key";
    char value[1021];
    for (size_t i = 0; i < 50; i++) {
        for (size_t j = 0; j < sizeof(value); j++)
            value[j] = 'a' + (j % 26);
        kv_set(key, value, sizeof(value), 0);
    }

    printf("The next `kv_set` will just fill the area\r\n");
    ThisThread::sleep_for(1000);
    res = kv_set(key, value, 509, 0);
    if (res != MBED_SUCCESS) {
        printf("kv_set fail error=%d\r\n", MBED_GET_ERROR_CODE(res));
        return 1;
    }

    printf("ok\r\n");
    return 0;
}

Impact of changes

This bug fix resolves an issue where valid key-value pairs could not be set when using the full storage capacity. There should be no negative impact on users; in fact, the behavior will now correctly reflect the intended design.

Documentation

None


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

Sorry, I cannot attach the test results as I only have a Raspberry Pi Pico environment.

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Copy link
Collaborator

@multiplemonomials multiplemonomials left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@multiplemonomials multiplemonomials merged commit df00bf8 into mbed-ce:master Apr 16, 2025
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants